Skip to content

Multi-platform & multi-version CI - #176

Merged
vaind merged 4 commits into
mainfrom
multiplatform-ci
Jan 25, 2021
Merged

vaind merged 4 commits into
mainfrom
multiplatform-ci

Conversation

@vaind

@vaind vaind commented Jan 25, 2021 •

Copy link
Copy Markdown
Contributor

@vaind vaind changed the title Multiplatform CI Multi-platform & multi-version CI Jan 25, 2021
@vaind
vaind force-pushed the multiplatform-ci branch 19 times, most recently from e9305c3 to 16e5905 Compare January 25, 2021 12:11
@vaind
vaind requested a review from greenrobot-team January 25, 2021 12:11

@greenrobot-team greenrobot-team left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some questions.

Comment thread lib/src/bindings/flatbuffers.dart Outdated
DynamicLibrary lib;
if (Platform.isWindows) {
// DynamicLibrary.process() is not available on Windows
lib = DynamicLibrary.open('msvcr100.dll');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this workaround coming from? Appears to be a C library. Does this require installation of Microsoft Visual C++ 2015 Redistributable (x64) like mentioned at https://docs.objectbox.io/java-desktop-apps#native-libraries? Should this be noted in a README?

The GitHub Windows instance has a bunch of versions installed, so I guess most are fine. https://github.com/actions/virtual-environments/blob/win19/20201210.0/images/win/Windows2019-Readme.md#microsoft-visual-c

@vaind vaind Jan 25, 2021 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this workaround coming from?

Nowhere in particular, I've just searched what libary defines memset on windows. Turns out it's mscvrt.dll or mscvr100.dll. Yes it may be fragile even though the library is pretty common, some users might not have it... I'll add another fallback layer

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add this in a comment then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, also added a comment

Comment thread lib/src/query/query.dart
Comment thread pubspec.yaml
Comment thread tool/pub.sh
@vaind
vaind merged commit d3045ee into main Jan 25, 2021
@vaind
vaind deleted the multiplatform-ci branch January 25, 2021 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI - test all supported dart/flutter versions CI on macOS and Windows

2 participants